From bbbf2dead8beabb51c967ebc05ba143636c050e8 Mon Sep 17 00:00:00 2001 From: Yehuda Katz + Carl Lerche Date: Tue, 10 Jun 2014 15:46:07 -0700 Subject: [PATCH] Isolate toml code and split up concerns --- libs/rust-toml | 2 +- src/bin/cargo-compile.rs | 2 +- src/bin/cargo-read-manifest.rs | 2 +- src/cargo/ops/cargo_compile.rs | 13 +- src/cargo/ops/cargo_read_manifest.rs | 24 ++-- src/cargo/ops/mod.rs | 2 +- src/cargo/sources/git/source.rs | 4 +- src/cargo/sources/path.rs | 4 +- src/cargo/util/toml.rs | 172 +++++++++++++++++++++++++++ 9 files changed, 198 insertions(+), 27 deletions(-) create mode 100644 src/cargo/util/toml.rs diff --git a/libs/rust-toml b/libs/rust-toml index 2f9e556c4..3eaa0ee4c 160000 --- a/libs/rust-toml +++ b/libs/rust-toml @@ -1 +1 @@ -Subproject commit 2f9e556c4ec1aa492884c951745699552494264b +Subproject commit 3eaa0ee4c81aa49bdcc98cad448ef3a6386d62af diff --git a/src/bin/cargo-compile.rs b/src/bin/cargo-compile.rs index 7710cead9..b4eae3c0f 100644 --- a/src/bin/cargo-compile.rs +++ b/src/bin/cargo-compile.rs @@ -37,5 +37,5 @@ fn execute(options: Options) -> CLIResult> { CLIError::new("Could not find Cargo.toml in this directory or any parent directory", Some(err), 102))) }; - ops::compile(root.as_str().unwrap().as_slice()).map(|_| None).to_cli(101) + ops::compile(&root).map(|_| None).to_cli(101) } diff --git a/src/bin/cargo-read-manifest.rs b/src/bin/cargo-read-manifest.rs index bb4a66918..32a3500a5 100644 --- a/src/bin/cargo-read-manifest.rs +++ b/src/bin/cargo-read-manifest.rs @@ -21,6 +21,6 @@ fn main() { } fn execute(options: Options) -> CLIResult> { - ops::read_manifest(options.manifest_path.as_slice()).map(|m| Some(m)) + ops::read_package(&Path::new(options.manifest_path.as_slice())).map(|m| Some(m)) .map_err(|err| CLIError::new(err.get_desc(), Some(err.get_detail()), 1)) } diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index a0311c9ea..bb69aa700 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -23,13 +23,12 @@ use sources::path::PathSource; use ops; use util::{other_error, CargoResult, Wrap}; -// TODO: manifest_path should be Path -pub fn compile(manifest_path: &str) -> CargoResult<()> { - log!(4, "compile; manifest-path={}", manifest_path); +pub fn compile(manifest_path: &Path) -> CargoResult<()> { + log!(4, "compile; manifest-path={}", manifest_path.display()); - let manifest = try!(ops::read_manifest(manifest_path)); + let package = try!(ops::read_package(manifest_path)); - debug!("loaded manifest; manifest={}", manifest); + debug!("loaded package; package={}", package); let configs = try!(config::all_configs(os::getcwd())); @@ -46,7 +45,7 @@ pub fn compile(manifest_path: &str) -> CargoResult<()> { let source = PathSource::new(paths); let summaries = try!(source.list().wrap("unable to list packages from source")); - let resolved = try!(resolve(manifest.get_dependencies(), &summaries).wrap("unable to resolve dependencies")); + let resolved = try!(resolve(package.get_dependencies(), &summaries).wrap("unable to resolve dependencies")); try!(source.download(resolved.as_slice()).wrap("unable to download packages")); @@ -54,7 +53,7 @@ pub fn compile(manifest_path: &str) -> CargoResult<()> { let package_set = PackageSet::new(packages.as_slice()); - try!(ops::compile_packages(&manifest, &package_set)); + try!(ops::compile_packages(&package, &package_set)); Ok(()) } diff --git a/src/cargo/ops/cargo_read_manifest.rs b/src/cargo/ops/cargo_read_manifest.rs index fb71c9a0c..95f1062d0 100644 --- a/src/cargo/ops/cargo_read_manifest.rs +++ b/src/cargo/ops/cargo_read_manifest.rs @@ -1,16 +1,16 @@ -use toml; -use core::Package; -use util::toml::toml_to_package; -use util::{CargoResult,human_error,toml_error}; +use std::io::File; +use util; +use core::{Package,Manifest}; +use util::{CargoResult,io_error}; -pub fn read_manifest(path: &str) -> CargoResult { - let root = try!(parse_from_file(path)); - toml_to_package(root, &Path::new(path)) +pub fn read_manifest(contents: &[u8]) -> CargoResult { + util::toml::to_manifest(contents) } -fn parse_from_file(path: &str) -> CargoResult { - toml::parse_from_file(path.clone()).map_err(|err| { - let err = toml_error("Couldn't parse Toml", err); - human_error("Cargo.toml is not valid Toml".to_str(), format!("path={}", path), err) - }) +pub fn read_package(path: &Path) -> CargoResult { + let mut file = try!(File::open(path).map_err(io_error)); + let data = try!(file.read_to_end().map_err(io_error)); + let manifest = try!(read_manifest(data.as_slice())); + + Ok(Package::new(&manifest, &path.dir_path())) } diff --git a/src/cargo/ops/mod.rs b/src/cargo/ops/mod.rs index fdd29a909..f5b6356ff 100644 --- a/src/cargo/ops/mod.rs +++ b/src/cargo/ops/mod.rs @@ -1,5 +1,5 @@ pub use self::cargo_compile::compile; -pub use self::cargo_read_manifest::read_manifest; +pub use self::cargo_read_manifest::{read_manifest,read_package}; pub use self::cargo_rustc::compile_packages; mod cargo_compile; diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index bc985e443..947f0e9a3 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -61,6 +61,6 @@ impl Source for GitSource { } fn read_manifest(path: &Path) -> CargoResult { - let joined = path.join("Cargo.toml"); - ops::read_manifest(joined.as_str().unwrap()) + let path = path.join("Cargo.toml"); + ops::read_package(&path) } diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index 452b0e2e6..f81061bf2 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -58,8 +58,8 @@ impl Source for PathSource { } fn read_manifest(path: &Path) -> CargoResult { - let joined = path.join("Cargo.toml"); - ops::read_manifest(joined.as_str().unwrap()) + let path = path.join("Cargo.toml"); + ops::read_package(&path) } fn display(paths: &[Path]) -> Vec { diff --git a/src/cargo/util/toml.rs b/src/cargo/util/toml.rs new file mode 100644 index 000000000..51e8221ff --- /dev/null +++ b/src/cargo/util/toml.rs @@ -0,0 +1,172 @@ +use toml; +use std::collections::HashMap; +use serialize::Decodable; + +use core::{Summary,Manifest,Target,Project,Dependency}; +use util::{CargoResult,Require,simple_human,toml_error}; + +pub fn to_manifest(contents: &[u8]) -> CargoResult { + let root = try!(toml::parse_from_bytes(contents).map_err(|_| + simple_human("Cargo.toml is not valid Toml"))); + + let toml = try!(toml_to_manifest(root).map_err(|_| + simple_human("Cargo.toml is not a valid Cargo manifest"))); + + toml.to_manifest() +} + +fn toml_to_manifest(root: toml::Value) -> CargoResult { + fn decode>(root: &toml::Value, path: &str) -> Result { + let root = match root.lookup(path) { + Some(val) => val, + None => return Err(toml::ParseError) + }; + toml::from_toml(root.clone()) + } + + let project = try!(decode(&root, "project").map_err(|e| toml_error("ZOMG", e))); + let lib = decode(&root, "lib").ok(); + let bin = decode(&root, "bin").ok(); + + let deps = root.lookup("dependencies"); + + let deps = match deps { + Some(deps) => { + let table = try!(deps.get_table().require(simple_human("dependencies must be a table"))).clone(); + + let mut deps: HashMap = HashMap::new(); + + for (k, v) in table.iter() { + match v { + &toml::String(ref string) => { deps.insert(k.clone(), SimpleDep(string.clone())); }, + &toml::Table(ref table) => { + let mut details = HashMap::::new(); + + for (k, v) in table.iter() { + let v = try!(v.get_str() + .require(simple_human("dependency values must be string"))); + + details.insert(k.clone(), v.clone()); + } + + let version = try!(details.find_equiv(&"version") + .require(simple_human("dependencies must include a version"))).clone(); + + deps.insert(k.clone(), DetailedDep(DetailedTomlDependency { + version: version, + other: details + })); + }, + _ => () + } + } + + Some(deps) + }, + None => None + }; + + Ok(TomlManifest { project: box project, lib: lib, bin: bin, dependencies: deps }) +} + +type TomlLibTarget = TomlTarget; +type TomlBinTarget = TomlTarget; + +/* + * TODO: Make all struct fields private + */ + +#[deriving(Encodable,PartialEq,Clone,Show)] +pub enum TomlDependency { + SimpleDep(String), + DetailedDep(DetailedTomlDependency) +} + +#[deriving(Encodable,PartialEq,Clone,Show)] +pub struct DetailedTomlDependency { + version: String, + other: HashMap +} + +#[deriving(Encodable,PartialEq,Clone)] +pub struct TomlManifest { + project: Box, + lib: Option>, + bin: Option>, + dependencies: Option>, +} + +impl TomlManifest { + pub fn to_manifest(&self) -> CargoResult { + + // Get targets + let targets = normalize(self.lib.as_ref().map(|l| l.as_slice()), self.bin.as_ref().map(|b| b.as_slice())); + + if targets.is_empty() { + debug!("manifest has no build targets; project={}", self.project); + } + + let mut deps = Vec::new(); + + // Collect the deps + match self.dependencies { + Some(ref dependencies) => { + for (n, v) in dependencies.iter() { + let version = match *v { + SimpleDep(ref string) => string.clone(), + DetailedDep(ref details) => details.version.clone() + }; + + deps.push(try!(Dependency::parse(n.as_slice(), version.as_slice()))) + } + } + None => () + } + + Ok(Manifest::new( + &Summary::new(&self.project.to_name_ver(), deps.as_slice()), + targets.as_slice(), + &Path::new("target"))) + } +} + +#[deriving(Decodable,Encodable,PartialEq,Clone,Show)] +struct TomlTarget { + name: String, + path: Option +} + +fn normalize(lib: Option<&[TomlLibTarget]>, bin: Option<&[TomlBinTarget]>) -> Vec { + log!(4, "normalizing toml targets; lib={}; bin={}", lib, bin); + + fn lib_targets(dst: &mut Vec, libs: &[TomlLibTarget]) { + let l = &libs[0]; + let path = l.path.clone().unwrap_or_else(|| format!("src/{}.rs", l.name)); + dst.push(Target::lib_target(l.name.as_slice(), &Path::new(path))); + } + + fn bin_targets(dst: &mut Vec, bins: &[TomlBinTarget], default: |&TomlBinTarget| -> String) { + for bin in bins.iter() { + let path = bin.path.clone().unwrap_or_else(|| default(bin)); + dst.push(Target::bin_target(bin.name.as_slice(), &Path::new(path))); + } + } + + let mut ret = Vec::new(); + + match (lib, bin) { + (Some(ref libs), Some(ref bins)) => { + lib_targets(&mut ret, libs.as_slice()); + bin_targets(&mut ret, bins.as_slice(), |bin| format!("src/bin/{}.rs", bin.name)); + }, + (Some(ref libs), None) => { + lib_targets(&mut ret, libs.as_slice()); + }, + (None, Some(ref bins)) => { + bin_targets(&mut ret, bins.as_slice(), |bin| format!("src/{}.rs", bin.name)); + }, + (None, None) => () + } + + ret +} -- 2.30.2